feat(otel): add complete OTEL configuration fields#143
Conversation
…stall Added missing OpenTelemetry configuration fields for all supported shells (bash, zsh, fish): - OTEL_METRIC_EXPORT_INTERVAL (10000ms) - OTEL_LOGS_EXPORT_INTERVAL (5000ms) - OTEL_LOG_USER_PROMPTS (enabled) - OTEL_METRICS_INCLUDE_SESSION_ID (enabled) - OTEL_METRICS_INCLUDE_VERSION (enabled) - OTEL_METRICS_INCLUDE_ACCOUNT_UUID (enabled) - OTEL_RESOURCE_ATTRIBUTES (with dynamic user.name, machine.name, team.id) This provides comprehensive telemetry configuration as requested in issue #142. Fixes #142 Co-authored-by: Le He <AnnatarHe@users.noreply.github.com>
Summary of ChangesHello @AnnatarHe, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request enhances the OpenTelemetry integration by adding a comprehensive set of configuration fields across bash, zsh, and fish shells. The changes aim to provide more detailed and configurable telemetry collection, including specific intervals for metric and log exports, user prompt logging, and richer resource attributes, addressing the requirements outlined in issue #142. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
Codecov Report❌ Patch coverage is
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 1 file with indirect coverage changes 🚀 New features to boost your workflow:
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request adds several new OpenTelemetry configuration fields for bash, zsh, and fish shells, which is great for providing more comprehensive telemetry. The implementation involves adding hardcoded strings for each shell type. While this works, it introduces code duplication and magic values. I've suggested using constants to improve maintainability. Additionally, for the fish shell, I've recommended using the idiomatic command substitution syntax to improve compatibility and adhere to best practices.
| "export OTEL_METRIC_EXPORT_INTERVAL=10000", | ||
| "export OTEL_LOGS_EXPORT_INTERVAL=5000", | ||
| "export OTEL_LOG_USER_PROMPTS=1", | ||
| "export OTEL_METRICS_INCLUDE_SESSION_ID=true", | ||
| "export OTEL_METRICS_INCLUDE_VERSION=true", | ||
| "export OTEL_METRICS_INCLUDE_ACCOUNT_UUID=true", | ||
| "export OTEL_RESOURCE_ATTRIBUTES=\"user.name=$(whoami),machine.name=$(hostname),team.id=shelltime\"", |
There was a problem hiding this comment.
These new OTEL configuration values are hardcoded and duplicated across the NewBashCCOtelEnvService, NewZshCCOtelEnvService, and NewFishCCOtelEnvService functions. This code duplication, especially between the bash and zsh implementations which are identical, can make future updates error-prone and tedious.
To improve maintainability, I recommend defining these values as constants at the package level. This centralizes the configuration, reduces the risk of inconsistencies, and makes the code easier to read and manage.
For example, you could define constants for the new settings:
const (
// ... existing constants
otelMetricExportInterval = "10000"
otelLogsExportInterval = "5000"
otelLogUserPrompts = "1"
otelMetricsIncludeSessID = "true"
// ... and so on
)Then, these constants can be used to construct the envLines slices for each shell. While a larger refactoring to eliminate the duplicated envLines slices might be out of scope for this PR, introducing constants for these new values would be a valuable improvement.
| "set -gx OTEL_METRICS_INCLUDE_SESSION_ID true", | ||
| "set -gx OTEL_METRICS_INCLUDE_VERSION true", | ||
| "set -gx OTEL_METRICS_INCLUDE_ACCOUNT_UUID true", | ||
| "set -gx OTEL_RESOURCE_ATTRIBUTES \"user.name=$(whoami),machine.name=$(hostname),team.id=shelltime\"", |
There was a problem hiding this comment.
For fish shell, the idiomatic syntax for command substitution is (command) rather than $(command). While $(command) is supported in modern versions of fish for POSIX compatibility, using the native (command) syntax is preferred. It ensures better compatibility with all fish versions and aligns with the shell's design principles.
| "set -gx OTEL_RESOURCE_ATTRIBUTES \"user.name=$(whoami),machine.name=$(hostname),team.id=shelltime\"", | |
| "set -gx OTEL_RESOURCE_ATTRIBUTES \"user.name=(whoami),machine.name=(hostname),team.id=shelltime\"" |
1 participant
Added missing OpenTelemetry configuration fields for all supported shells (bash, zsh, fish):
This provides comprehensive telemetry configuration as requested in issue #142.
Fixes #142
Generated with Claude Code